Skip to content

test: collapse test/isolated/ into test/lib and test/commands#840

Merged
BYK merged 1 commit intomainfrom
byk/collapse-test-isolated
Apr 24, 2026
Merged

test: collapse test/isolated/ into test/lib and test/commands#840
BYK merged 1 commit intomainfrom
byk/collapse-test-isolated

Conversation

@BYK
Copy link
Copy Markdown
Member

@BYK BYK commented Apr 24, 2026

Follow-up to #839. With bun test --isolate giving each file a fresh module graph, the test/isolated/ split that existed to work around mock.module() registry pollution (oven-sh/cli#258) is no longer needed.

Merged into existing files

Per-feature tests now live with their natural counterparts:

From To
isolated/brew-upgrade.test.ts test/lib/upgrade.test.ts
isolated/log-view-prompt.test.ts test/commands/log/view.test.ts
isolated/login-reauth.test.ts test/commands/auth/login.test.ts
isolated/project-delete-confirm.test.ts test/commands/project/delete.test.ts
isolated/set-commits-auto.test.ts test/lib/api/releases.test.ts
isolated/set-commits-auto-no-remote.test.ts test/lib/api/releases.test.ts (merged together — getRepositoryName is now a controllable mock())
isolated/dsn/project-root.test.ts test/lib/dsn/project-root.test.ts

Pattern: mock.module() at the top of the merged file, followed by a dynamic await import() for the code under test so the mocked bindings win. Other static imports stay as-is.

Also unblocks the previously-deferred stub in test/lib/api/releases.test.ts that said "setCommitsAuto tests live in test/isolated/ because they require mock.module() for git helpers".

Moved as sibling files

Two isolated files tested the same subjects as existing files but with incompatible mocking strategies. Under --isolate they coexist safely as siblings:

From To
isolated/resolve-target.test.ts test/lib/resolve-target.mocked.test.ts
isolated/delta-upgrade.test.ts test/lib/delta-upgrade.mocked.test.ts

The existing resolve-target.test.ts and delta-upgrade.test.ts use real DB + mocked fetch; the .mocked. siblings stub every collaborator via mock.module().

New files (no prior counterpart)

Three isolated DSN helpers had no target file in test/lib/dsn/:

  • isolated/dsn/errors.test.tstest/lib/dsn/errors.test.ts
  • isolated/dsn/fs-utils.test.tstest/lib/dsn/fs-utils.test.ts
  • isolated/dsn/resolver.test.tstest/lib/dsn/resolver.test.ts

Removed scopedFetchMock helper

test/lib/sentry-client.test.ts previously wrapped all its fetch mocks with scopedFetchMock(marker, handler) to filter out foreign fetch calls that leaked from other test files (CI run 24835339085). Under bun test --isolate each file gets a fresh globalThis.fetch, so cross-file leakage is eliminated. The 6 call sites now use mockFetch() directly; the helper and its urlOf() dependency are gone (~30 lines).

Other cleanups

  • Removed test/isolated/ directory
  • Updated package.json test:unit to drop test/isolated from the argument list
  • test/lib/dsn/project-root.test.ts: noopSpan stub now includes setAttributes (production code calls both setAttribute and setAttributes)
  • Replaced () => {} test stubs with named noop functions to satisfy lint/suspicious/noEmptyBlockStatements without per-line biome-ignore comments

Verification

  • bun run test:unit — 5920 pass / 0 fail across 253 files (was 260 — merges consolidated 12 files into 5 new + 7 existing)
  • ✅ 3 consecutive stress runs clean
  • bun run typecheck, bun run lint

Net diff: +1219 / −3830 (most "deletions" are file moves; real code reduction is the scopedFetchMock removal plus deduplication from merging).

Follow-up to #839. With `bun test --isolate` giving each file a fresh
module graph, the test/isolated/ split that existed to work around
`mock.module()` pollution (oven-sh/cli#258) is no longer needed.

## Merged into existing files

Per-feature tests moved into their natural counterparts:

- isolated/brew-upgrade.test.ts              → test/lib/upgrade.test.ts
- isolated/log-view-prompt.test.ts           → test/commands/log/view.test.ts
- isolated/login-reauth.test.ts              → test/commands/auth/login.test.ts
- isolated/project-delete-confirm.test.ts    → test/commands/project/delete.test.ts
- isolated/set-commits-auto.test.ts          → test/lib/api/releases.test.ts
- isolated/set-commits-auto-no-remote.test.ts → test/lib/api/releases.test.ts
                                                (merged as a single
                                                setCommitsAuto describe;
                                                getRepositoryName is now
                                                a controllable `mock()`)
- isolated/dsn/project-root.test.ts          → test/lib/dsn/project-root.test.ts

Pattern: `mock.module()` at the top of the merged file, followed by a
dynamic `await import()` for the code under test so the mocked bindings
win. The rest of the file's static imports stay as-is.

Also unblocks a previously-deferred stub in `test/lib/api/releases.test.ts`
that said "setCommitsAuto tests live in test/isolated/ because they
require mock.module() for git helpers".

## Moved as sibling files

Two isolated files tested the same subjects as existing test files but
with incompatible mocking strategies. Under `--isolate` they're safe to
keep as siblings since module state doesn't leak:

- isolated/resolve-target.test.ts  → test/lib/resolve-target.mocked.test.ts
- isolated/delta-upgrade.test.ts   → test/lib/delta-upgrade.mocked.test.ts

Both existing `resolve-target.test.ts` and `delta-upgrade.test.ts` use
real DB + mocked fetch; the `.mocked.` siblings stub out every
collaborator via `mock.module()`. The two styles can coexist in the
same directory now that module graphs are per-file.

## New files (no prior counterpart)

Three isolated DSN helpers had no target file in test/lib/dsn/:

- isolated/dsn/errors.test.ts     → test/lib/dsn/errors.test.ts
- isolated/dsn/fs-utils.test.ts   → test/lib/dsn/fs-utils.test.ts
- isolated/dsn/resolver.test.ts   → test/lib/dsn/resolver.test.ts

## Removed `scopedFetchMock` helper

`test/lib/sentry-client.test.ts` previously wrapped all its fetch mocks
with `scopedFetchMock(marker, handler)` to filter out foreign fetch
calls that leaked from other test files (CI run 24835339085). Under
`bun test --isolate` each file gets a fresh `globalThis.fetch`, so
cross-file leakage is eliminated. The 6 call sites now use `mockFetch()`
directly; the helper and its `urlOf()` dependency are gone (~30 lines).

## Other cleanups

- Removed `test/isolated/` directory
- Updated `package.json` `test:unit` to drop `test/isolated` from the
  argument list (files now live under `test/lib/` and `test/commands/`)
- `test/lib/dsn/project-root.test.ts`: `noopSpan` stub now includes
  `setAttributes` (production code calls both `setAttribute` and
  `setAttributes`)
- Replaced several `() => {}` test stubs with named `noop` functions
  to satisfy `lint/suspicious/noEmptyBlockStatements` without per-line
  biome-ignore comments

## Verification

- `bun run test:unit` — 5920 pass / 0 fail across 253 files (was 260 —
  merges consolidated 12 files into 5 new + 7 existing)
- 3 consecutive stress runs clean
- `bun run typecheck`, `bun run lint`
- `bun run lint:fix` applied one formatting tweak
@github-actions
Copy link
Copy Markdown
Contributor

Codecov Results 📊

5920 passed | Total: 5920 | Pass Rate: 100% | Execution Time: 0ms

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

✅ Patch coverage is 100.00%. Project has 13025 uncovered lines.
✅ Project coverage is 75.16%. Comparing base (base) to head (head).

Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    75.10%    75.16%    +0.06%
==========================================
  Files          284       284         —
  Lines        52482     52437       -45
  Branches         0         0         —
==========================================
+ Hits         39412     39412         —
- Misses       13070     13025       -45
- Partials         0         0         —

Generated by Codecov Action

@BYK BYK merged commit 54deec4 into main Apr 24, 2026
23 checks passed
@BYK BYK deleted the byk/collapse-test-isolated branch April 24, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant